-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new multiple ice sheet functionality #140
Conversation
… roundoff from nov06
@DeniseWorthen @uturuncoglu - can you both please verify as part of the code review that the changes here will not effect nems or hafs. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have verified that all RTs pass for ufs-model (fe5a943) substituting this branch for CMEPS.
@mvertens i am testing with HAFs now. I'll let you know soon. |
@DeniseWorthen - thanks so much for verifying this so quickly!!!! |
@DeniseWorthen - what repository did you use for the s2s testing - so that I can fill this in the PR. |
@uturuncoglu - thanks for the quick response. |
@jedwards4b i am getting following error when I try to build the HAFS model with data components
I check the commit history and internal PIO update (feature/hafs_cdeps) was already merged with master. At least, it shown like that https://github.com/ESCOMP/CMEPS/tree/feature/hafs_cdeps but there is no "merged" sign if you look at the list of branches. I am not sure why I am getting error related with PIO_SHORT. @jedwards4b Do you have any idea? Maybe it is not merged. |
@jedwards4b it seems feature/hafs_cdeps is old and not used branch and we merged feature/hafs_cdeps_fixed with #121 to update PIO |
@jedwards4b it seems that PIO shipped with master is not updated version.I think following PR revert all PIO changes back https://github.com/ESCOMP/CMEPS/pull/122/files |
@mvertens I tested in my ufs-weather fork @ develop; hash fe5a943 but with CMEPS replaced w/ your branch |
@mvertens @DeniseWorthen @rsdunlapiv @jedwards4b @danrosen25 I think that managing internal PIO shipped with CMEPS is creating lots of confusion and it is hard to maintain. Maybe we could define it as external dependency in HAFS application also like S2S. By this way, we could get rid of entire CMEPS/nems/lib directory (but I think we still need to have CMEPS/nems/util, please confirm @DeniseWorthen). Let me know what do you think? If it is okay for you I could approve this PR and work on it. |
@uturuncoglu We will still need nems/util but I agree removing pio makes sense. |
integer, public, parameter :: comprof = 6 | ||
integer, public, parameter :: compwav = 7 | ||
integer, public, parameter :: compglc1 = 8 | ||
integer, public, parameter :: ncomps = 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? Why number of components is still 8?
Because for now we are only using one ice sheet and we need to generalize
the implementation. This will be done when the antarctic ice sheet comes in.
…On Mon, Nov 30, 2020 at 4:43 PM Ufuk Turunçoğlu ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In mediator/esmFlds.F90
<#140 (comment)>:
> - integer, public, parameter :: compatm = 2
- integer, public, parameter :: complnd = 3
- integer, public, parameter :: compocn = 4
- integer, public, parameter :: compice = 5
- integer, public, parameter :: comprof = 6
- integer, public, parameter :: compwav = 7
- integer, public, parameter :: compglc = 8
+ integer, public, parameter :: compmed = 1
+ integer, public, parameter :: compatm = 2
+ integer, public, parameter :: complnd = 3
+ integer, public, parameter :: compocn = 4
+ integer, public, parameter :: compice = 5
+ integer, public, parameter :: comprof = 6
+ integer, public, parameter :: compwav = 7
+ integer, public, parameter :: compglc1 = 8
+ integer, public, parameter :: ncomps = 8
Is this correct? Why number of components is still 8?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#140 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4XCE4Q6HOVY5DP2GVR56TSSQU3HANCNFSM4UG7XEQA>
.
--
Mariana Vertenstein
CESM Software Engineering Group Head
National Center for Atmospheric Research
Boulder, Colorado
Office 303-497-1349
Email: mvertens@ucar.edu
|
Point to Mariana Vertenstein's branch for ESCOMP/CMEPS#140
* adds multiple ice sheet functionality (ESCOMP#140) * adds post phases for performance enhancement (ESCOMP#146) * adds ocn->glc (land ice) coupling at multiple levels (ESCOMP#148)
Description of changes
Permit GLC (CISM, XGLC) to send multiple ice sheets on different meshes
Specific notes
This PR does the following:
As an example for nems:
As an example for cesm:
The new wrapper has the following syntax:
Contributors other than yourself, if any:
CMEPS Issues Fixed (include github issue #):
Resolves #131
Are changes expected to change answers?
Any User Interface Changes (namelist or namelist defaults changes)?
Testing performed if application target is CESM:(either UFS-S2S or CESM testing is required):
Testing performed if application target is UFS-S2S:
Hashes used for testing: